Skip to content

feat/email-registration - #212

Closed
islandbitcoin wants to merge 5 commits into
mainfrom
feat/email-registration
Closed

feat/email-registration#212
islandbitcoin wants to merge 5 commits into
mainfrom
feat/email-registration

Conversation

@islandbitcoin

@islandbitcoin islandbitcoin commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Summary

Implements email-only authentication flow for new user registration, allowing users to sign up with just an email address (no phone required).

Key Changes:

  • New GraphQL mutations: newUserEmailRegistrationInitiate and newUserEmailRegistrationValidate
  • Kratos identity schema for email-only accounts (email_no_password_v0)
  • Account upgrade path: device accounts can upgrade to email accounts
  • Registration payload validation for email-based flows

How It Works

  1. Client calls newUserEmailRegistrationInitiate with email
  2. Backend creates Kratos identity, sends OTP via recovery flow
  3. Client calls newUserEmailRegistrationValidate with flowId + code
  4. Backend validates code, creates account if new, returns auth token

Files Changed (43 files, +855/-216)

Core Implementation:

  • src/graphql/public/root/mutation/new-user-email-registration-*.ts - GraphQL mutations
  • src/app/authentication/email.ts - Email authentication logic
  • src/services/kratos/auth-email-no-password.ts - Kratos integration
  • src/app/accounts/create-account.ts - Account creation updates
  • src/app/accounts/upgrade-device-account.ts - Device → Email upgrade
  • src/domain/authentication/registration-payload-validator.ts - Validation logic
  • dev/ory/kratos.yml - Kratos configuration

Test Infrastructure:

  • Fixed TypeScript errors from main rebase (branded types, mock updates, API changes)

Known Issues / Follow-up Tickets

🔴 HIGH PRIORITY (security review needed)

  1. Account Enumeration Vulnerability

    • Location: src/services/kratos/auth-email-no-password.ts lines 73-78
    • Issue: createIdentityForEmailRegistration() reveals whether email is already registered
    • Mitigation: Should return consistent response regardless of email existence
  2. Missing TOTP Flow Completion

    • Location: newUserEmailRegistrationValidate returns totpRequired but no follow-up verification
    • Issue: If TOTP is required, there's no mutation to complete the flow
    • Needs: Document expected client behavior or implement TOTP verification step
  3. Race Condition in Account Creation

    • Location: new-user-email-registration-validate.ts lines 63-78
    • Issue: Concurrent code validations could create duplicate accounts
    • Mitigation: Add distributed lock or database constraint

🟡 MEDIUM PRIORITY (tech debt)

  1. Inconsistent Error Handling - "dead branch" error message (line 162-163)
  2. Hardcoded Schema ID - Should use SchemaIdType.EmailNoPasswordV0 enum
  3. Known Account Enumeration TODO - FIXME at lines 415-419 not addressed
  4. Missing Transaction Handling - Multi-step operations in upgrade-device-account.ts

🟢 LOW PRIORITY

  1. Unused import pattern in validate mutation
  2. Undocumented GraphQL complexity value (120)
  3. Direct Kratos DB access (documented workaround for issue #3163)

Testing

  • TypeScript compiles (yarn tsc --noEmit - 0 errors)
  • Integration tests (pre-existing failures unrelated to this PR)

Checklist

  • Code follows project conventions
  • TypeScript errors resolved
  • Security issues documented for follow-up
  • Ready for review

@islandbitcoin
islandbitcoin requested a review from brh28 September 16, 2025 20:00
@islandbitcoin islandbitcoin self-assigned this Sep 16, 2025
@islandbitcoin islandbitcoin added the enhancement New feature or request label Sep 16, 2025
@islandbitcoin
islandbitcoin force-pushed the feat/email-registration branch from 81894da to 66e733f Compare September 17, 2025 15:54

@brh28 brh28 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already the following definition in the graphql schema, which appears to the same or similar to what's being done in this PR:

  userEmailRegistrationInitiate(input: UserEmailRegistrationInitiateInput!): UserEmailRegistrationInitiatePayload!
  userEmailRegistrationValidate(input: UserEmailRegistrationValidateInput!): UserEmailRegistrationValidatePayload!

Comment thread dev/ory/email-template.html
Comment thread src/graphql/admin/schema.graphql Outdated
@islandbitcoin

Copy link
Copy Markdown
Contributor Author

There's already the following definition in the graphql schema, which appears to the same or similar to what's being done in this PR:

  userEmailRegistrationInitiate(input: UserEmailRegistrationInitiateInput!): UserEmailRegistrationInitiatePayload!
  userEmailRegistrationValidate(input: UserEmailRegistrationValidateInput!): UserEmailRegistrationValidatePayload!

These definitions do not allow for a email registration without an existing account. The choice was to modify these, to handle both cases (with account and without account) or create a new schema definition for new accounts.

@islandbitcoin
islandbitcoin force-pushed the feat/email-registration branch from 66e733f to 90f293e Compare October 17, 2025 22:08
@islandbitcoin
islandbitcoin requested a review from brh28 October 17, 2025 22:33
@islandbitcoin

Copy link
Copy Markdown
Contributor Author

@brh28 please review

// so that if one fails, the other is rolled back

// 1. Update user record with email (deviceId is preserved via spread)
const userUpdated = await UsersRepository().findById(userId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 60-65 can be consolidated to a single database update. The function would be something like:

addEmail: (userId, email) => db.updateOne( { userId }, { $set: { email })

userId: UserId
email: EmailAddress
}): Promise<Account | RepositoryError> => {
// TODO: ideally both 1. and 2. should be done in a transaction,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe there's a way to make this atomic unless we completely rewrite our data model


import { createAccountWithEmailIdentifier } from "@app/accounts"

export const createAccountFromEmailRegistrationPayload = async ({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this function being called?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nowhere! apparently its leftover from a previous attempt at getting this to work. Removed the file and the export reference

@brh28 brh28 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a code walk through on this one? I'm having a hard time following

@brh28

brh28 commented Oct 30, 2025

Copy link
Copy Markdown
Contributor

Related to: #237

@islandbitcoin

Copy link
Copy Markdown
Contributor Author

@brh28 I think this is ready for a code review again, since we patched the orphaned accounts issue.

@islandbitcoin islandbitcoin changed the title [feat] account creation using only email (no phone) feat/email-registration Dec 29, 2025
const accountsRepo = AccountsRepository()
let account = await accountsRepo.findByUserId(kratosUserId)

if (account instanceof Error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should be checking for a specific response, such as AccountNotFoundError

} else {
// Create new identity with email
const createIdentityBody = {
credentials: { password: { config: { password } } },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does password have a value?

}

// Send OTP code via recovery flow
const { data: recoveryFlow } = await kratosPublic.createNativeRecoveryFlow()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verify we want createNativeRecoveryFlow rather than createNativeRegistrationFlow

type: [String],
},
deviceId: {
email: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emails are already stored in Kratos. Is there any reason not to use the postrgres database here?

  - New GraphQL mutations: newUserEmailRegistrationInitiate and newUserEmailRegistrationValidate for email-only account creation
  - Kratos integration: Successfully using email recovery flow for OTP delivery
  - Account creation: Fixed critical bug where accounts weren't being created after validation
  - Code cleanup: Removed all debug console.log statements

  ✅ Key Changes Made

  1. Fixed validation logic - Changed from checking User existence to Account existence
  2. Proper account creation - Creates account with wallets when none exists
  3. Clean production code - Removed debug statements for production readiness
This commit consolidates all TypeScript fixes required after rebasing
the email-registration feature branch on main:

- Update core Account and Wallet mocks with mandatory properties (npub, lnurlp)
- Adapt to upstream API changes (@ory/client IdentityState → IdentityStateEnum)
- Fix test infrastructure mock type conversions and exports
- Replace deleted BTC wallet functions with USD equivalents
- Use factory functions for branded types (OnChainAddress, FractionalCentAmount)
- Fix OffersManager constructability and CSV export method names
- Add type casts for transaction and wallet type incompatibilities

Result: yarn tsc --noEmit returns 0 errors
@islandbitcoin
islandbitcoin force-pushed the feat/email-registration branch from fdea49c to 204ac45 Compare January 27, 2026 12:28
@brh28
brh28 self-requested a review January 27, 2026 15:25
islandbitcoin added a commit that referenced this pull request Jan 27, 2026
…lidation deferral

- Add ValidationError import to redeem-invite.ts to fix TypeScript errors
- Document that email validation is deferred until email-only registration
  feature is available (see PR #212)
@islandbitcoin

Copy link
Copy Markdown
Contributor Author

Closed as deferred

islandbitcoin added a commit that referenced this pull request Jul 31, 2026
…lidation deferral

- Add ValidationError import to redeem-invite.ts to fix TypeScript errors
- Document that email validation is deferred until email-only registration
  feature is available (see PR #212)
islandbitcoin added a commit that referenced this pull request Aug 1, 2026
…462)

* feat: Implement referral system with Email/SMS/WhatsApp invites
Add comprehensive invite-friend feature allowing users to invite friends via Email, SMS, or WhatsApp.

**User-Facing Features:**
- Create invites: Users can send invites via Email (SendGrid), SMS, or WhatsApp (Twilio)
- Redeem invites: New users can redeem invites within 1 hour of account creation
- Preview invites: Unauthenticated endpoint to preview invite before registration
- Rate limiting: 10 invites/day per user, 3 invites/day per target contact (Redis-based)
- 24-hour invite expiration with Firebase Dynamic Links support

**Admin Features:**
- View invite details with inviter/redeemer information
- List and filter invites by status and inviter
- Paginated invite queries

**Technical Implementation:**
- MongoDB schema for invite tracking with secure token hashing (SHA-256)
- Notification service supporting Email, SMS, and WhatsApp
- Contact validation for email/phone formats
- Deep linking support via Firebase Dynamic Links
- Comprehensive test coverage (unit & integration tests)

**Security:**
- Tokens are 40-character random strings with only SHA-256 hash stored
- Contact verification ensures invite sent to correct recipient
- Account age validation (< 1 hour) for new user redemption
- Self-redemption prevention

* fix: improve invite feature consistency and remove code duplication

- Fix rate limit key inconsistency between admin functions and rate
  limiter service (use RateLimitPrefix constants)
- Refactor GraphQL createInvite mutation to use @app/invite layer
  instead of duplicating business logic
- Add index on redeemedById field in invite schema for query performance
- Make new-user invite redemption window configurable via
  NEW_USER_INVITE_WINDOW_HOURS constant (default 24 hours, was 1 hour)
- Standardize token generation to use 20-byte (40-char) tokens

* fix: improve type safety in invite feature

- Add INVITE_TOKEN_LENGTH constant (40 chars) to domain
- Add InviteToken branded type with checkedToInviteToken validator
- Fix token length check in app/invite/redeem-invite.ts (was 64, should be 40)
- Replace magic number checks with typed validation in GraphQL mutations
- Use checkedToAccountId instead of unsafe `as AccountId` cast in invite-preview

* fix(invite): add missing ValidationError import and document email validation deferral

- Add ValidationError import to redeem-invite.ts to fix TypeScript errors
- Document that email validation is deferred until email-only registration
  feature is available (see PR #212)

* chore(invite): regenerate admin GraphQL SDL after rebase

Rebasing feat/invite onto main took main's admin schema.graphql (--ours) during
conflict resolution; write-sdl regenerates it to include the invite admin types
(AdminInvite, invitesList, inviteById). Public SDL already carried the invite
types via clean auto-merge. Full `yarn build` compiled cleanly, verifying the
rebase conflict resolutions typecheck.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* test(invite): add backend unit tests for invite/refer feature

80 tests / 8 suites, fully mocked (no infra): domain validation + invite
constants/token checks, hash/token generation, app-layer create-invite,
redeem-invite, rate-limits, queries, and admin ops. Covers success + error
paths (invalid contact, rate-limited, duplicate, expired, self-redeem, etc.).

GraphQL resolver wiring + Redis-backed rate-limiter left for test/flash/integration
(need infra). Note: redeem-invite's reward-crediting is still a TODO (redemption
only flips status to ACCEPTED).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* feat(invite): tiered referral reward payout on Bridge KYC approval

Implements the referral reward the invite feature only stubbed. When an invited
user's Bridge KYC is approved (they gain a US account), both the inviter and the
invitee are paid a tiered USD reward, funded from a dedicated 'rewards' wallet.

- New 'rewards' account role (AccountRoles, mongoose enum, AdminRole); assign it
  to the funding account via a direct mongo write. Resolved with
  AccountsRepository().findByRole.
- Tiered amount by global referral sequence (atomic counter): 1-100 -> $5,
  101-600 -> $2.50, 601+ -> $1. Ops-tunable via the new referralReward config
  block (default DISABLED, so nothing pays until a rewards wallet is assigned).
- Trigger: the once-only pending->approved transition in the Bridge KYC webhook
  (CAS-guarded). Payout via intraledgerPaymentSendWalletIdForUsdWallet.
- Idempotent + fail-closed: atomic claim on the invite, per-party
  inviter/inviteeRewardedAt, never double-pays; a failed/partial payout is
  recorded (rewardStatus) for manual reconciliation and never throws into or
  blocks KYC approval. Admin visibility via new AdminInvite reward fields.

Tests: 23 unit (9 tier boundaries + 14 payout paths incl. idempotency, partial,
failed, disabled). tsc-check clean; admin SDL regenerated.
(Also fixes a latent tsc-check type error in the admin invite spec's mock.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* fix(invite): make the feature CI-green (scope-map, lint, module-load)

The invite feature was never CI-green (never merged), so PR #462 surfaced
several gaps plus two regressions from the reward payout:

- api-key scope-map: register createInvite (authed mutation) as BLOCKED so the
  deny-by-default completeness test passes. redeemInvite/invitePreview are in
  the unauthed schema block, so they are (correctly) not authed root fields.
- award-referral-reward: lazy-import send-intraledger inside payParty so merely
  importing @app/invite no longer pulls the IBEX client (baseLogger.child at
  init) — was breaking kyc.spec + create-invite.spec at module load.
- ops-events-hooks.spec @config mock: add getInviteCreateAttemptLimits/
  getInviteTargetAttemptLimits (domain/rate-limit evaluates them at load).
- prettier/eslint: format the never-linted invite files; drop unused imports
  (InviteToken; redeem-invite mutation dead imports); type two anys in
  services/notification.

Gates: eslint 0 errors, tsc-check + tsc-check-noimplicitany clean, full unit
suite 1304 passed / 0 failed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* fix(invite): harden reward payout per review findings

- Stuck-claim recovery: the atomic claim stamps rewardClaimedAt, and any
  unexpected throw after the claim downgrades it to 'failed' with a
  rewardError instead of stranding an invisible 'processing' row.
- IBEX Pending is a distinct non-terminal 'pending' rewardStatus (new enum
  value). Per-party timestamps are still set for pending parties — fail-closed,
  a re-run can never double-pay — but ops now sees it needs re-checking
  instead of it being counted as terminally paid.
- Payouts fund from the rewards account's USDT wallet first (the active cash
  wallet), falling back to USD, and recipients are resolved strictly in the
  funding wallet's currency (send-intraledger rejects cross-currency sends).
- Tier fail-safe: a schedule missing its unbounded sentinel pays 0 past the
  last bound instead of silently over-paying forever.

Tests: award spec 14->18 (post-claim throw, pending semantics, wallet
preference/currency-match, claim stamp), tier fail-safe boundaries.
Gates: scoped jest 130/130, tsc-check + noimplicitany + eslint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* fix(invite): close re-review findings — one-reward-per-invitee invariant + 8 more

F1 (blocking): one reward per invitee, enforced in three layers — redemption
rejects a second redemption per account, a unique partial index on redeemedById
closes the race (duplicate-key treated as already-redeemed), and the award path
skips accounts that already have any rewardStatus so a Bridge KYC
approved->under_review->approved flap can never pay a second accumulated invite.

F2: the post-claim catch preserves payment evidence — a throw mid-payout now
records partial with the paid party's timestamp instead of failed-with-nothing
(manual reconciliation can no longer double-pay a paid party).
F3: redeemInvite moved to the authed block + scope-map BLOCKED (was reachable
by read-scoped API keys via the unauthed shield gap). SDL unchanged.
F4: raw invite tokens and Twilio auth-token fragments no longer logged.
F5: revoked/EXPIRED-status invites rejected at redeem + preview, independent
of the date check.
F6: Timestamp scalar accepts ISO strings again (parseInt regression silently
turned admin cutover scheduledAt into 1970); pure digits = epoch seconds,
invalid input errors. Pinned by a new scalar spec.
F7: admin invitesList — ObjectId cast for the pipeline filter (was always
empty) and validated _id-cursor pagination (was parseInt(after,16) nonsense).
F8: a failed invite notification deletes the invite and returns an error
instead of burning the contact's 24h dup-window with nothing sent.
F9: dead app-layer redeem module deleted; the LIVE resolver now has a 14-case
spec (token/window/phone/EMAIL/self/race/revoked/success paths).

Full unit suite 157 suites / 1326 passed / 0 failed; tsc-check,
noimplicitany, eslint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* fix: 'unparseable' -> 'unparsable' (typos CI)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

* fix(invite): check ACCEPTED before the date-expiry flip on redeem

A post-expiry replay of an already-redeemed invite's token used to overwrite
ACCEPTED with EXPIRED — stranding the pending reward and, now that accounts
are limited to one redemption ever, permanently costing the account its
referral. Reorder the checks; regression test pins ACCEPTED + no save.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C7z7o9J18BWbUnYJcemMtg

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Dread <bobodread@bobodread.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants